-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: Extend vscode 'run' command with optional mode argument for run… #20077
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ning test(s) or bin at keyboard cursor
To test executing the 'run' command with 'cursor' as supplied argument one can for example add the following to keybindings.json: |
editors/code/src/ctx.ts
Outdated
if (name === "run") { | ||
// Special case: support optional argument for `run` | ||
callback = (mode?: "cursor") => { | ||
return commands.run(this, mode)(); | ||
}; | ||
} else { | ||
// we asserted that `client` is defined | ||
callback = factory.enabled(this); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary, Cmd
(the type of callback) is already correctly typed in allowing more parameters. Instead we need to change the enabled
field here
rust-analyzer/editors/code/src/main.ts
Line 170 in ab9e7bd
run: { enabled: commands.run }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah In fact that doesn't need changes either. Just having edited the signature of command.run
should suffice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it. I've tried to only revert the special casing in command factory (ctx.ts). However it seems adding the special callback as you suggest in your first comment is necessary to make it work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah indeed, enabled
is not a Cmd
but a function that returns a Cmd
👍
…update main.createCommands instead
Thanks! |
…ning test(s) or bin at keyboard cursor
Fixes #14805